[NO-TICKET] Preact discovery #2368
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… one This simplifies it by stripping out the Sass stuff. We could possibly add some CSS back in later, but right now I'm focused on the JS part of it. Some stats: - `react-app` produces a bundle weighing 188 KB - `preact-app` produces a bundle weighing 76 KB
Note that the additional babel config breaks Storybook, but if we end up going this route, we can update our Storybook config to work with preact
If I were doing this for real, I'd make a separate web components bundle so there are no side effects in the npm package. Problem right now is being able to pass `children` to web components. Going to try https://github.com/jahilldev/component-elements/tree/main/packages/preactement#readme next based on [this issue thread](preactjs/preact-custom-element#41)
The conversion of the tag contents into the `children` prop still doesn't seem to work, even though the [preactement readme](https://github.com/jahilldev/component-elements/tree/main/packages/preactement) shows an example of it. I'm wondering if it's an issue of support for the latest version of Preact. This library hasn't been updated in nine months. Nope, that can't be it, because their package.json says it wants a peer dependency of `"preact": ">=10"`, which is what we're on.
I've inspected the Alert render function while it's rendering the web component, and `children` does have a value (it's a rendered React/Preact element, so I can't see what's in it), but nothing ends up in the final output, and that content in between the custom element tags is just sitting there below all the rendered html
to see if I could find out what the contents of the children prop are. I don't think there's anything in it. I stepped through Preact's render function to the part where it reconciles differences with the DOM. My content wasn't there, and I know that doesn't actually prove anything, but I just have a feeling it isn't a problem with anything that happens from the Alert render call down. I do think it's a problem with the value of the `children` prop that is passed to the Alert instance
Just had to move the script to the end of `<body>` so the full DOM is loaded at the time when the initial value of children is being queried. See the commit messages for b709c89, 98f78f9, and 90bc645 for details about the problem. In summary, the `children` prop isn't being correctly populated. My most recent experiment in seeing where it goes wrong: print out the value of `this.inner` from [this line of preactement](https://github.com/jahilldev/component-elements/blob/main/packages/preactement/src/define.ts#L134). I actually printed it from [the parseHtml function](https://github.com/jahilldev/component-elements/blob/8c5666ccaad6cbeda73b77a5d0eb430a09071494/packages/preactement/src/parse.ts#L17). The result is a blank printout. Aha! I printed out [the return value of `getDocument`](https://github.com/jahilldev/component-elements/blob/8c5666ccaad6cbeda73b77a5d0eb430a09071494/packages/preactement/src/parse.ts#L17), which was empty `<body></body>` tags, which led me to test moving the bundle script tag out of `<head>` and to the end of `<body>`, and it worked!!!!!
Some pass and a lot of snapshot tests fail, as to be expected. Will need to go through these and determine if it makes sense to even test snapshots at all for Preact if we expect so many differences in the resultant DOM ``` Test Suites: 29 failed, 55 passed, 84 total Tests: 66 failed, 636 passed, 702 total Snapshots: 56 failed, 122 passed, 178 total ```
Unfortunately I'd need to change the `framework` config for storybook, and you [can't currently mix frameworks](storybookjs/storybook#3889). Going to revert this but wanted a record of the journey
This reverts commit a557886.
Until we've gone through each component to make web components versions—and ones that take complex props could be a lot of work—we probably want to have two different bundles. My next commit, therefore, will be to build two different bundles
Unfortunately we have no way as it is now to pass boolean props. I tried setting `analytics="true"`, but the Preact component receives that as a string. We will probably need a layer of abstracting components that translate between attributes and props in a more intentional way. There are also components like MonthPicker with array props that will need a lot of work in this area
3 tasks
|
Closing, as it has now been implemented for real |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This branch is the result of a discovery into whether we can repackage our existing React components with the lighter-weight Preact framework in order to save on bundle sizes. The RFC discussing this discovery work can be found here.
How to test
yarn install && yarn buildexamplesthat use Preact- and web-component versions of our design system components in different ways